Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check for webgl2 validation errors and catch issues #3296

Merged
merged 4 commits into from
Dec 20, 2022
Merged

Check for webgl2 validation errors and catch issues #3296

merged 4 commits into from
Dec 20, 2022

Conversation

cwfitzgerald
Copy link
Member

Checklist

  • Run cargo clippy.
  • Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections

Helps with #3282

Description

Testing
Explain how this change is tested.

@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2022 •
edited
Loading

Codecov Report

Merging #3296 (b10f9d3) into master (2480eff) will decrease coverage by 0.00%.
The diff coverage is 6.25%.

@@            Coverage Diff             @@
##           master    #3296      +/-   ##
==========================================
- Coverage   65.63%   65.63%   -0.01%     
==========================================
  Files          82       82              
  Lines       39479    39488       +9     
==========================================
+ Hits        25914    25918       +4     
- Misses      13565    13570       +5
Impacted Files Coverage Δ
wgpu/src/lib.rs 51.73% <0.00%> (+0.30%) ⬆️
wgpu/src/backend/direct.rs 56.03% <8.33%> (-0.41%) ⬇️
wgpu-core/src/binding_model.rs 60.86% <0.00%> (-1.24%) ⬇️
wgpu-core/src/device/mod.rs 66.75% <0.00%> (+0.11%) ⬆️
wgpu-core/src/validation.rs 59.01% <0.00%> (+0.13%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@haraldreingruber
Copy link
Contributor

Hey, nice work 🤙

I also spent a bit of time experimenting with catching the WebGL errors...
Your implementation looks much cleaner, and seems to fit better with the wgpu (test) architecture.
Still, I wanted to share my current implementation, for example, the error logging might be something you might find useful to cherry-pick?

Just an idea...

@cwfitzgerald cwfitzgerald mentioned this pull request Dec 20, 2022
Open
@cwfitzgerald
Copy link
Member Author

@haraldreingruber Normally I would be totally game for expanded error messages - but in this particular case it can be a bit misleading. Because there are potentially so many errors registered during execution, combined with how horrid the GL error scheme is, adding more information when we don't really know much could leave people going down the wrong path. GL errors will already come through through the console, so we'll get some level of error messages.

@haraldreingruber
Copy link
Contributor

@cwfitzgerald makes totally sense. Thanks for the explanation.

@cwfitzgerald cwfitzgerald marked this pull request as ready for review December 20, 2022 20:48
@cwfitzgerald cwfitzgerald merged commit 6b6bc69 into gfx-rs:master Dec 20, 2022
@cwfitzgerald cwfitzgerald deleted the webgl2-tests branch December 20, 2022 20:51
@teoxoy teoxoy mentioned this pull request Jan 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nical nical nical approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cwfitzgerald @codecov-commenter @haraldreingruber @nical